Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate creation & usage of OxDispatcher #3759

Merged
merged 1 commit into from
May 10, 2024
Merged

Conversation

adamw
Copy link
Member

@adamw adamw commented May 10, 2024

Whenever I read OxDispatcher code I got a bit confused, and got worried that the captured Ox scope is going to be used when calling .runAsync on another thread.

Separating creation of the dispatcher (where the scope needs to be captured) from scheduling computations (where we just need to send a message to an actor on a channel) maybe will clarify this for future users (and me ;) )

@adamw adamw requested a review from kciesielski May 10, 2024 07:08
@adamw adamw changed the title Separate creationg & usage of OxDispatcher Separate creation & usage of OxDispatcher May 10, 2024
@kciesielski
Copy link
Member

Is the confusion caused by seeing a new OxDispatcher and not expecting that the constructor captures Ox scope in the constructor call site?

@adamw
Copy link
Member Author

adamw commented May 10, 2024

No, the confusion comes from the fact that when I navigate to OxDispathcer from usage-site (where .runAsync) is invoked, I see a captured Ox capability and this looks suspicious. Capturing capabilities is always suspicious ;) Here the capability is only captured for the creation, so it's more obvious that you only need it to create the actor (and not to call the actor)

@adamw adamw merged commit 2aa60e0 into master May 10, 2024
26 checks passed
@adamw adamw deleted the ox-dispatcher-cleanup branch May 10, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants